Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[preview]Add PIC support in the srv6 VPN scenario. #16879

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zice312963205
Copy link
Contributor

What I did
[HLD] BGP PIC HLD

Why I did it
This PR implements the separation of nh_srv6-related information, which was originally stored in the nexthop, in the srv6-vpn scenario. It generates a new pic nexthop that contains only the next-hop forwarding information. Meanwhile, the original nexthop containing nh_srv6 is used as pic context information and indexed separately.
This is used to achieve fast switching of nexthop group members by identifying the corresponding members in the nexthop group based on route changes, and then performing operations on the members.

This PR is temporarily for review purposes. Documentation and test cases will be added later, and a complete PR will be resubmitted.

@frrbot frrbot bot added the bgp label Sep 20, 2024
@zice312963205 zice312963205 changed the title Add PIC support in the srv6 VPN scenario. [preview]Add PIC support in the srv6 VPN scenario. Sep 20, 2024
This PR implements the separation of nh_srv6-related information, which was originally stored in the nexthop, in the srv6-vpn scenario. It generates a new pic nexthop that contains only the next-hop forwarding information. Meanwhile, the original nexthop containing nh_srv6 is used as pic context information and indexed separately.

Signed-off-by: hanyu.zly&freddy <[email protected]>
Copy link
Contributor

@cscarpitta cscarpitta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments.

The commit should be broken into multiple commits (bgpd, lib, zebra, ...)
https://docs.frrouting.org/projects/dev-guide/en/latest/workflow.html#commit-guidelines

@@ -2289,6 +2289,9 @@ ssize_t netlink_route_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx

/* Table corresponding to this route. */
table_id = dplane_ctx_get_table(ctx);
if (fpm)
table_id = dplane_ctx_get_vrf(ctx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we overriding the table ID with the VRF ID here? Isn't this a breaking change?

@@ -65,6 +65,8 @@ struct mgmt_be_client *mgmt_be_client;
/* Route retain mode flag. */
int retain_mode = 0;

bool fpm_pic_nexthop = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this variable used for? It is set to true here and never changed. It seems that it is unnecessary and can be removed?


ctx->u.rinfo.nhe.id = nhe->id;
ctx->u.rinfo.nhe.old_id = 0;
if (nhe->pic_nhe) {
/**/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the empty comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants